fix(window): persist and restore window size and position across restarts by DysektAI · Pull Request #16 · tarkovtracker-org/RatScanner

DysektAI · 2026-07-24T23:54:11Z

Root cause

Only LastWindowPositionX/Y were persisted, and only at exit. PageSwitcher's constructor always called ResetWindowSize() (1080×720), so every restart discarded the user's size and re-anchored the position to a stale coordinate pair — the window shifted exactly as described in #11.

Fix

New LastWindowWidth / LastWindowHeight config keys, round-tripped alongside the existing position keys (backwards compatible; missing keys stay unset).
Startup restores saved size + position, but the saved caption strip must still intersect an attached display's working area (IsVisibleOnAnyScreen). If the monitor layout changed since last run, the window falls back to default placement instead of appearing off-screen.
Exit persists normal-mode bounds from whichever state the app is in:
- minimal UI → pre-minimal restore bounds
- maximized → RestoreBounds
- otherwise → live geometry
- first-run closed while still in minimal UI keeps any previously saved size (no trustworthy normal geometry exists yet)
Exiting minimal UI with no in-session restore bounds (app started directly into minimal mode) now also restores persisted bounds.

Validation

New WindowBoundsPersistenceTests: config round-trip, missing-key defaults, off-screen rejection, on-screen acceptance (209 passed total).
csharpier clean. Manual smoke: resize/move → restart → same bounds; unplug a monitor → falls back to default placement.

Fixes #11

Summary by cubic

Persist and restore window size and position across restarts. Adds DPI‑correct on‑screen checks so the window won’t resurrect off‑screen after monitor or scale changes.

Bug Fixes
- Added LastWindowWidth/LastWindowHeight config keys; backward‑compatible when missing.
- Restore only when the caption strip intersects an attached display; convert Screen.WorkingArea from physical pixels to WPF logical units using per‑monitor DPI from WindowsGameDisplayService and ignore unreliable DPI reads.
- On close, persist normal‑mode bounds: minimal UI uses pre‑minimal restore; maximized/minimized use RestoreBounds; otherwise use live geometry. Skip saving minimal‑UI coordinates when no normal bounds exist; save once during shutdown.
- Starting directly in minimal UI, switching to full UI restores persisted normal bounds; invalid/off‑screen saves fall back to default placement.
- New helpers/tests: TryGetRestorableBounds, TryGetPersistableBounds, physical→logical working‑area conversion, and high‑DPI visibility validation.

^{Written for commit 84993d6. Summary will update on new commits.}

Greptile Summary

This PR persists and restores window size (new LastWindowWidth/LastWindowHeight config keys) alongside the existing position keys, and adds monitor-layout validation so the window falls back to default placement when the saved position lands off all currently-attached displays.

RestoreWindowBounds / PersistWindowBounds replace the previous inline save/restore, correctly handling the maximized, minimal-UI, and first-run-minimal-mode edge cases.
IsVisibleOnAnyScreen validates the caption strip against Screen.AllScreens working areas before restoring position; however, the app runs PerMonitorV2 DPI awareness (set in Program.Main) so Screen.WorkingArea returns physical pixels while WPF window coordinates are device-independent units — these diverge at non-100% DPI and can cause off-screen positions to pass the check.
Four new xUnit tests cover the config round-trip, missing-key defaults, and off/on-screen detection; the on-screen test computes its center position from physical-pixel screen dimensions passed as WPF logical units, which is only accurate at 100% DPI.

Confidence Score: 3/5

The core persistence logic and edge-case handling are sound, but the off-screen guard compares WPF logical coordinates against physical-pixel screen bounds in a PerMonitorV2 process, which means the guard can silently accept off-screen positions on high-DPI machines.

The coordinate mismatch in IsVisibleOnAnyScreen directly undermines the fix for issue #11: on any machine running at a non-100% DPI scale, a window that was on a now-disconnected monitor can still pass the visibility check and be restored off-screen. The config round-trip and the minimal-UI/maximized branching logic are well-structured and backwards-compatibility is clean, but the DPI bug affects the one guard added specifically to prevent the reported problem.

Files Needing Attention: src/App/PageSwitcher.xaml.cs — specifically IsVisibleOnAnyScreen and the fallback position-save path in PersistWindowBounds; tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs — the on-screen acceptance test.

Important Files Changed

Filename	Overview
src/App/PageSwitcher.xaml.cs	Adds RestoreWindowBounds/PersistWindowBounds helpers and IsVisibleOnAnyScreen; logic is well-structured but IsVisibleOnAnyScreen compares WPF logical units against physical-pixel Screen.WorkingArea under PerMonitorV2, causing off-screen detection to fail at non-100% DPI.
src/App/RatConfig.cs	Adds LastWindowWidth/LastWindowHeight static fields with matching read/write calls; backwards-compatible (missing keys use 0 default), no config version bump needed since new keys degrade gracefully.
tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs	New test file covering config round-trip, missing-key defaults, and off/on-screen detection; the on-screen test uses physical-pixel screen coordinates as WPF logical units, which is only accurate at 100% DPI.

Sequence Diagram

sequenceDiagram
    participant Config as RatConfig
    participant PS as PageSwitcher
    participant Screen as Screen.AllScreens

    note over PS: Startup
    PS->>Config: LoadConfig()
    PS->>PS: ResetWindowSize() (1080x720 defaults)
    PS->>Config: Read LastWindowWidth/Height/X/Y
    PS->>PS: RestoreWindowBounds()
    PS->>Screen: IsVisibleOnAnyScreen(left, top, width, height)
    Screen-->>PS: true / false
    alt position on-screen
        PS->>PS: "Left=saved, Top=saved, Width=saved, Height=saved"
    else position off-screen
        PS->>PS: keep default placement
    end

    note over PS: Enter Minimal UI
    PS->>PS: "_restoreBounds = RestoreBounds"
    PS->>PS: ShowMinimalUI()

    note over PS: Exit Minimal UI (ShowUI)
    alt _restoreBounds non-empty
        PS->>PS: restore from _restoreBounds
    else started in minimal mode (no in-session bounds)
        PS->>PS: ResetWindowSize() + RestoreWindowBounds()
    end

    note over PS: Exit / Close
    PS->>PS: PersistWindowBounds()
    alt closing from minimal UI and _restoreBounds empty
        PS->>Config: "LastWindowPositionX/Y = minimal UI Left/Top"
    else normal geometry
        PS->>Config: "LastWindowPositionX/Y/Width/Height = bounds"
    end
    PS->>Config: SaveConfig()

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/App/PageSwitcher.xaml.cs:142-159
**Coordinate-system mismatch breaks off-screen detection at non-100% DPI**

`Program.Main` calls `Application.SetHighDpiMode(HighDpiMode.PerMonitorV2)`, which means `System.Windows.Forms.Screen.WorkingArea` returns **physical device pixels**. WPF's `Window.Left`/`Top`/`Width`/`Height` — the values passed as `left`, `top`, `width`, `height` — are in **WPF device-independent units** (96 DPI base). At 150% DPI, 1 WPF unit = 1.5 physical pixels.

Concrete failure: a user's secondary 1920 px-wide monitor at 150% DPI is 1280 WPF units wide. They position the window at WPF `Left = 1300` (barely off the right edge). Config saves `1300`. They unplug the monitor. On next launch `IsVisibleOnAnyScreen(1300, …)` computes `grabLeft ≈ 1340` (WPF units) and compares against `area.Right = 1920` (physical pixels). `1340 < 1920` is true, so the window is accepted as on-screen and restored to a physically inaccessible position — the exact regression issue #11 was meant to cure.

One approach: obtain the device pixel ratio from the `PresentationSource` (available on `this` in the instance context) and scale the WPF coordinates before comparing, or convert each `Screen.WorkingArea` rectangle from physical pixels to WPF units by dividing by the appropriate DPI scale factor.

### Issue 2 of 3
tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs:78-87
**Test uses physical-pixel coordinates as WPF logical units**

`Screen.PrimaryScreen.WorkingArea.Width` returns physical device pixels under `PerMonitorV2`. The test feeds those values directly to `IsVisibleOnAnyScreen` as if they were WPF device-independent units. At 100% DPI (typical CI machine) the two scales are identical so the test passes, but on a 150% DPI developer machine `area.Width` could be 2880 px while the actual WPF-unit width of the same monitor is 1920. The computed `left = (2880-1080)/2 = 900` looks plausible but represents a position physically at 1350 px — very different from where the WPF window would actually appear. The test exercises a path through the function but does not accurately model the coordinates that `RestoreWindowBounds` passes at runtime.

### Issue 3 of 3
src/App/PageSwitcher.xaml.cs:583-598
**Fallback writes the minimal-UI position as the normal-window position**

When `bounds.IsEmpty` (first-run closed while still in minimal UI, meaning `_restoreBounds` was never set to valid normal-mode geometry), the fallback saves `Left`/`Top` of the *minimal UI* overlay as `LastWindowPositionX/Y`. On the next launch, if `LastWindowMode` is `Normal`, `RestoreWindowBounds` will position the full-size window at the compact overlay's coordinates (which could be a corner of the screen). The position is then validated by `IsVisibleOnAnyScreen`, so it at least won't restore off-screen, but the window will appear at an ergonomically surprising location. Skipping the position update in this branch (leaving `LastWindowPositionX/Y` as `int.MinValue`) would fall back to OS default placement, which is likely preferable to anchoring the normal window at an arbitrary minimal-UI corner.

_{Reviews (1): Last reviewed commit: "fix(window): persist and restore window ..." | Re-trigger Greptile}

Greptile also left 3 inline comments on this PR.

…arts Only the window position was saved (and only at exit); the constructor always reset to 1080x720, so every restart lost the user's size and shifted the window (issue #11). - Add LastWindowWidth/LastWindowHeight config keys, round-tripped with the existing position keys. - Restore saved size and position at startup, but only when the saved caption strip still intersects an attached display's working area; a changed monitor layout falls back to default placement instead of resurrecting the window off-screen. - Save normal-mode bounds at exit: from minimal UI use the pre-minimal restore bounds, from maximized use RestoreBounds, otherwise live geometry. First-run minimal closes keep any previously saved size. - Exiting minimal UI with no in-session restore bounds (started directly into minimal mode) now also restores persisted bounds. Regression tests cover config round-trip, missing-key defaults, and on/off-screen visibility validation. Fixes #11

coderabbitai · 2026-07-24T23:54:21Z

📝 Walkthrough

Walkthrough

Window geometry persistence now includes size and position, validates minimum dimensions and display visibility during restoration, handles minimal-to-normal transitions, and selects validated bounds during shutdown. Tests cover configuration round trips, missing size defaults, DPI conversion, and screen visibility.

Changes

Window bounds persistence

Layer / File(s)	Summary
Geometry persistence and restoration `src/App/RatConfig.cs`, `src/App/Display/WindowsGameDisplayService.cs`, `src/App/PageSwitcher.xaml.cs`	`RatConfig` loads and saves persisted width and height values. `PageSwitcher` restores saved geometry only when dimensions meet minimums and the position is visible on an attached display, using DPI-aware working-area conversion.
Lifecycle geometry handling `src/App/PageSwitcher.xaml.cs`	Minimal-to-normal transitions restore persisted bounds when session bounds are unavailable, while shutdown persists validated bounds based on the current UI state and prevents duplicate exit handling.
Persistence and visibility validation `tests/RatScanner.Tests/WindowBoundsPersistenceTests.cs`	Tests cover size and position round trips, unset defaults for missing size keys, DPI-aware visibility checks, restorable-size fallback, and persistence rules for minimized or minimal UI states.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant PageSwitcher
  participant RatConfig
  participant WindowsGameDisplayService
  User->>PageSwitcher: launch or return to normal UI
  PageSwitcher->>RatConfig: read saved window geometry
  PageSwitcher->>WindowsGameDisplayService: read display DPI and working areas
  WindowsGameDisplayService-->>PageSwitcher: return logical display bounds
  PageSwitcher->>PageSwitcher: validate and apply visible bounds
  User->>PageSwitcher: close application
  PageSwitcher->>PageSwitcher: select and validate persistable bounds
  PageSwitcher->>RatConfig: save window geometry

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: persisting and restoring window size and position across restarts.
Description check	✅ Passed	The description is detailed and directly matches the implemented window persistence and restoration changes.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/window-bounds-persistence

_{Comment @coderabbitai help to get the list of available commands.}

DysektAI · 2026-07-27T08:57:17Z

Addressed the fresh Kilo review finding in eb4f444: persistence now receives the minimal-mode restore rectangle and WPF RestoreBounds as separate inputs, so minimized/maximized exits use the real state restore geometry even when minimal UI was never entered. The minimized regression test now explicitly supplies an empty minimal restore rectangle and a valid WPF state restore rectangle. Release build and full tests remain clean (216 passed, 5 fixture-dependent skips).